-
Notifications
You must be signed in to change notification settings - Fork 421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
@W-16362973: [iOS] Add QR Code Login Support in MSDK #3759
@W-16362973: [iOS] Add QR Code Login Support in MSDK #3759
Conversation
libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Login/SFLoginViewController.h
Show resolved
Hide resolved
…CodeLoginEnabled`. Functional P.O.C.)
65f19ee
to
38ba3b0
Compare
…ic Handling Of Query String Vs. Fragment)
…thHelper` With QR Code Log In Parameters)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #3759 +/- ##
==========================================
- Coverage 43.43% 43.32% -0.11%
==========================================
Files 223 223
Lines 20587 20637 +50
==========================================
Hits 8941 8941
- Misses 11646 11696 +50
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Here's the APEX for user agent flow:
|
Here's the APEX for web server flow. Note, @wmathurin and I only have one page and controller in place. We've been toggling the APEX content as we switch between tests. We could scale that so it is more convenient.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's my self-review notes and code walk-though.
libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Common/SalesforceSDKManager.h
Show resolved
Hide resolved
libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Login/SFLoginViewController+QrCodeLogin.swift
Show resolved
Hide resolved
libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Login/SFLoginViewController+QrCodeLogin.swift
Outdated
Show resolved
Hide resolved
libs/SalesforceSDKCore/SalesforceSDKCore/Classes/OAuth/SFOAuthCoordinator.m
Outdated
Show resolved
Hide resolved
libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Login/SFLoginViewController.h
Show resolved
Hide resolved
libs/SalesforceSDKCore/SalesforceSDKCore/Classes/OAuth/SFOAuthCoordinator.m
Show resolved
Hide resolved
libs/SalesforceSDKCore/SalesforceSDKCore/Classes/OAuth/SFOAuthCoordinator.m
Show resolved
Hide resolved
libs/SalesforceSDKCore/SalesforceSDKCore/Classes/OAuth/SFOAuthCoordinator.m
Show resolved
Hide resolved
libs/SalesforceSDKCore/SalesforceSDKCore/Classes/UserAccount/SFUserAccountManager+Internal.h
Show resolved
Hide resolved
…r Bridge Parameters On Authorization Failure)
@bbirman and @wmathurin - I added a commit to match MSDK Android's addition of a UI Bridge Front Door URL reset on both authentication success and failure. That will aid users if they fail a QR code log in and the app gives them the option to return back to the web view or if they use a QR code with different parameters. The app would need to stop the current authentication and re-call one of the auth helper log in methods as well if the user cancelled from the QR code log in. |
🎸 Ready For Review! 🥁
This adds the MSDK logic needed to support QR Code Log In to MSDK iOS, much like its Android counterpart.
This includes support for both web server flow and user agent flow in the QR code. Most of the Quip written by @wmathurin still applies, but be aware the APEX code has changed significantly for web server flow. I'll post the updated APEX here as well.
I added a really detailed code walkthrough in my self-review. I highly recommend reading it in detail this time, since this is my personal first foray into this tenured section of the authentication logic. Do share thoughts on the pattern I used for connecting the new parameters to and then using them in the existing authentication logic.
Be sure to see the follow-up pull request, W-16171422: [iOS] Update Native Login Sample App to include QR Code Login Flow.
Thanks for reading, as always.